refactor: slightly improve typing for cqlengine by VincentRPS · Pull Request #197 · scylladb/python-driver

VincentRPS · 2022-12-22T11:31:23Z

This PR slightly improves typing for the CQLEngine.
I am having a bit of trouble though, because of the weird way Model and Queries are done,
its very hard to properly type model.object.* functions properly. Any ideas?

VincentRPS · 2022-12-22T11:57:44Z

seems like there's no way to properly type model.object.* functions so, as for what I see, this PR should be ready

fruch · 2022-12-27T18:59:59Z

cassandra/cqlengine/models.py

    def _inst_get_connection(self):
        return self._connection or self.__connection__

+    def __getitem__(self, s: slice | int) -> M | list[M]:


this is a bit more then adding types, this add new functionality.
I think it should come on it's own commit, with the reasoning why it's needed

ah yeah, the reason this was added was mostly since I found proper typing for a model on ModelQuerySet to be nearly impossible

fruch · 2022-12-27T19:01:41Z

@VincentRPS I've let CI run on this change, but I think it might fail on python2

for reasons™️ we still need to support python2, so this might need to wait a bit, for when we can drop python2 support completely

fruch · 2022-12-29T22:14:52Z

@VincentRPS

seems like it's failing with python3.7

2022-12-27T19:04:08.5435241Z ______________ ERROR collecting tests/unit/cqlengine/test_udt.py _______________
2022-12-27T19:04:08.5435657Z /project/tests/unit/cqlengine/test_udt.py:18: in <module>
2022-12-27T19:04:08.5436038Z     from cassandra.cqlengine.models import Model
2022-12-27T19:04:08.5436576Z /tmp/tmp.ALd4n4nucG/venv/lib/python3.7/site-packages/cassandra/cqlengine/models.py:24: in <module>
2022-12-27T19:04:08.5437001Z     from cassandra.cqlengine import query
2022-12-27T19:04:08.5437514Z /tmp/tmp.ALd4n4nucG/venv/lib/python3.7/site-packages/cassandra/cqlengine/query.py:1073: in <module>
2022-12-27T19:04:08.5437975Z     T = TypeVar('T', 'ModelQuerySet')
2022-12-27T19:04:08.5438399Z /opt/python/cp37-cp37m/lib/python3.7/typing.py:545: in __init__
2022-12-27T19:04:08.5438786Z     raise TypeError("A single constraint is not allowed")
2022-12-27T19:04:08.5439150Z E   TypeError: A single constraint is not allowed

fruch · 2022-12-29T22:16:37Z

@VincentRPS

and as expected python2 doesn't support type annotations:

______________ ERROR collecting tests/unit/cqlengine/test_udt.py _______________
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/_pytest/python.py:507: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/py/_path/local.py:704: in pyimport
    __import__(modname)
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/_pytest/assertion/rewrite.py:304: in load_module
    exec(co, mod.__dict__)
tests/unit/cqlengine/test_udt.py:18: in <module>
    from cassandra.cqlengine.models import Model
E     File "/home/runner/work/python-driver/python-driver/cassandra/cqlengine/models.py", line 90
E       def __get__(self, obj: Any, model: "BaseModel"):
E                            ^
E   SyntaxError: invalid syntax
=============================== warnings summary ===============================

VincentRPS · 2023-01-11T10:32:21Z

@VincentRPS

and as expected python2 doesn't support type annotations:

______________ ERROR collecting tests/unit/cqlengine/test_udt.py _______________
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/_pytest/python.py:507: in _importtestmodule
    mod = self.fspath.pyimport(ensuresyspath=importmode)
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/py/_path/local.py:704: in pyimport
    __import__(modname)
/opt/hostedtoolcache/Python/2.7.18/x64/lib/python2.7/site-packages/_pytest/assertion/rewrite.py:304: in load_module
    exec(co, mod.__dict__)
tests/unit/cqlengine/test_udt.py:18: in <module>
    from cassandra.cqlengine.models import Model
E     File "/home/runner/work/python-driver/python-driver/cassandra/cqlengine/models.py", line 90
E       def __get__(self, obj: Any, model: "BaseModel"):
E                            ^
E   SyntaxError: invalid syntax
=============================== warnings summary ===============================

hmm, seeing that I'd have to say it'd be better waiting until py2 support is dropped

fruch · 2023-01-11T10:44:35Z

thanks @VincentRPS, I'll mark this one as pending for now

fruch · 2023-07-18T14:47:05Z

@VincentRPS, now that we remove python2 support, we can try this again

fruch · 2023-07-18T14:49:07Z

@VincentRPS, I would recommend send it also to the upstream cassandra (don't know when they would except it)

fruch · 2023-07-18T15:45:26Z

Seems like those changes are also breaking python3.7, so for now we can't merge those

VincentRPS · 2023-08-01T13:14:42Z

@fruch I thought I had already replied here but I guess I didn't, it should work with 3.7 now since I did a little fix, though I could be mistaken.

fruch · 2023-08-01T13:22:41Z

@fruch I thought I had already replied here but I guess I didn't, it should work with 3.7 now since I did a little fix, though I could be mistaken.

I'll let CI run again

fruch · 2024-05-30T05:11:08Z

@fruch I thought I had already replied here but I guess I didn't, it should work with 3.7 now since I did a little fix, though I could be mistaken.

I'll let CI run again

I've rebase this one, and triggered CI again

Lorak-mmk · 2024-06-18T14:34:27Z

@VincentRPS could you send this to upstream? It would be better to do it there: less conflicts when merging + we don't really touch cqlengine code, so it's hard to do a proper review.

fruch reviewed Dec 27, 2022

View reviewed changes

fruch added the enhancement New feature or request label Jan 9, 2023

fruch added the pending label Jan 11, 2023

fruch removed the pending label Jul 18, 2023

fruch force-pushed the fix-engine-typing branch from f897cbe to aa440e5 Compare July 18, 2023 14:47

fruch added the pending label Jul 18, 2023

VincentRPS added 3 commits May 30, 2024 08:10

refactor: slightly improve typing

d47b63c

chore: remove redundant queryset mention

e7efcf0

fix: error related to typevar

5f35565

fruch force-pushed the fix-engine-typing branch from 989b943 to 5f35565 Compare May 30, 2024 05:10

Lorak-mmk self-assigned this Jun 1, 2024

Lorak-mmk self-requested a review June 1, 2024 09:27

Lorak-mmk removed their assignment Jul 31, 2024

Lorak-mmk removed their request for review September 20, 2024 13:19

Merge branch 'master' into fix-engine-typing

d32d44e

Conversation

VincentRPS commented Dec 22, 2022

Uh oh!

VincentRPS commented Dec 22, 2022

Uh oh!

fruch Dec 27, 2022

Choose a reason for hiding this comment

Uh oh!

VincentRPS Jan 11, 2023

Choose a reason for hiding this comment

Uh oh!

fruch commented Dec 27, 2022

Uh oh!

fruch commented Dec 29, 2022

Uh oh!

fruch commented Dec 29, 2022

Uh oh!

VincentRPS commented Jan 11, 2023

Uh oh!

fruch commented Jan 11, 2023

Uh oh!

fruch commented Jul 18, 2023

Uh oh!

fruch commented Jul 18, 2023

Uh oh!

fruch commented Jul 18, 2023

Uh oh!

VincentRPS commented Aug 1, 2023

Uh oh!

fruch commented Aug 1, 2023

Uh oh!

fruch commented May 30, 2024

Uh oh!

Lorak-mmk commented Jun 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants